Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support omitting empty fields on map encoders #77

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

whyrusleeping
Copy link
Owner

No description provided.

for _, f := range gti.Fields {
if f.OmitEmpty {
err := doTemplate(w, f, `
if t.{{ .Name }} == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't go's definition of "empty".

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

You can:

  • Do nothing if you don't care.
  • Use reflect.Value.IsZero (some runtime cost).
  • Have a bunch of logic to define the "zeros" for different kinds.
  • Define per-field zero values as "variables" in codegen (using reflect.Zero).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so, there's an easier way.

var {{.Name}}Zero {{.Type}}
if t.{{.Name}} == {{.Name}}Zero {
    ...
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... I was getting kinda lazy since this fix was three layers down the rabbit hole. I'd like to avoid runtime costs so i think generator time type checking is the right thing to do


fmt.Fprintf(w, `
if _, err := cw.Write(cbg.CborEncodeMajorType(cbg.MajMap, uint64(%d - emptyFieldCount))); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: define fieldCount, then subtract one by one for every empty field.

@whyrusleeping
Copy link
Owner Author

will fix the zero type stuff at a later time

@whyrusleeping whyrusleeping merged commit c09a31a into master Dec 13, 2022
@whyrusleeping whyrusleeping deleted the feat/omitempty branch December 13, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants